Skip to content

feat(studies-ui): restore lost Trials + Convergence list columns#438

Merged
SoundMindsAI merged 4 commits into
mainfrom
feat_studies_list_trial_convergence_columns
Jun 3, 2026
Merged

feat(studies-ui): restore lost Trials + Convergence list columns#438
SoundMindsAI merged 4 commits into
mainfrom
feat_studies_list_trial_convergence_columns

Conversation

@SoundMindsAI
Copy link
Copy Markdown
Owner

Summary

Adds Trials + Convergence columns to the /studies list table — answering the operator request to surface trial count on the studies page.

This is a restoration of lost work, not a new feature. The backend already returns trial_count + convergence_verdict on StudySummary (shipped in feat_studies_convergence_visibility, PR #421, Story 1.1 / b90d5477). The matching frontend columns (Story 1.2, commit ed5ca276) were authored + reviewed but never reached main — they were dropped in the PR #421/#422 rebase that de-duplicated Epic 1 commits. So the two fields sat returned-but-unsurfaced; the /studies list ran with only its original 6 columns.

This PR cherry-picks ed5ca276 to restore the reviewed original (better than a fresh reimplementation: header tooltipKey pattern, hideable, satisfies Record<ConvergenceVerdict> exhaustiveness, the study.trial_count glossary key, a dedicated unit test, + an E2E spec):

  • Trials column — trial_count (the non-baseline count, resolved server-side via the batched count_trials_for_studies aggregate — no N+1).
  • Convergence column — convergence_verdictConverged / Improving / Too few trials badge, em-dash when null (in-flight / <5 complete trials). Reuses CONVERGENCE_VERDICT_VALUES + the convergence_verdict glossary key.

Bug caught during restoration

The header tooltipKey renders an <InfoTooltip> in the column header, whose Radix Tooltip needs a TooltipProvider ancestor. The app shell (layout.tsx) provides one in prod, but page.test.tsx renders <StudiesPage> in isolation — so it now wraps in TooltipProvider, mirroring the layout. That ed5ca276 didn't already carry this fix confirms it never ran the full suite on main (consistent with being lost pre-merge).

Doc-drift corrected

state_history.md and the feat_studies_convergence_visibility implementation plan both marked Story 1.2 as shipped via PR #421. They didn't. Both now carry a CORRECTION (2026-06-03) annotation pointing here.

Guide regen

Guide 06's 01-studies-list.png was stale (old 6-column list). Regenerated against the populated stack — now shows the TRIALS column (50/200/200/15/…) + the CONVERGENCE column (CONVERGED / TOO FEW TRIALS badges). 03-create-study-modal + 04-study-detail regenerated in lockstep; walkthrough video promoted.

Test coverage

Layer File Cases
UI vitest studies-table-convergence-column.test.tsx (restored) 7
UI vitest studies-table-ceiling-badge.test.tsx (fixture extended) 5
UI vitest studies/page.test.tsx (TooltipProvider + mock fields) 5
E2E studies-convergence-columns.spec.ts (restored) smoke-gated

1039 vitest green; tsc + build clean; freshness gate clean (no backend change). No migration.

Test plan

  • pnpm --dir ui test → 1039/1039
  • pnpm --dir ui typecheck + pnpm --dir ui build clean
  • pnpm --dir ui lint → 0 errors
  • Freshness gate: git status clean after canonical regen
  • Guide 06 regenerated against populated stack (columns visible with real data)
  • CI green

🤖 Generated with Claude Code

SoundMindsAI and others added 3 commits June 3, 2026 17:08
The studies list backend already returns trial_count + convergence_verdict
(feat_studies_convergence_visibility, PR #421 Story 1.1 / b90d547), but
the Story 1.2 *frontend* columns (commit ed5ca27) never reached main —
they were dropped in the PR #421/#422 rebase that de-duplicated Epic 1
commits. So the fields sat returned-but-unsurfaced; the /studies list ran
with only its original 6 columns.

This restores ed5ca27 by cherry-pick (the reviewed original is better
than a fresh reimplementation — header tooltipKey pattern, hideable,
`satisfies Record<ConvergenceVerdict>` exhaustiveness, the study.trial_count
glossary key, a dedicated unit test, and an E2E spec):

- studies-table.column-config.tsx: Trials column (trial_count) +
  Convergence column (convergence_verdict → Converged / Improving /
  Too few trials badge, em-dash when null). Both use header tooltipKey
  + are hideable.
- studies-table-convergence-column.test.tsx (restored): 7 cases.
- studies-convergence-columns.spec.ts (restored): E2E.
- glossary.ts: study.trial_count + convergence_verdict short entries.

Caught a gap the lost commit never exercised: the header tooltipKey
renders an <InfoTooltip> in the column header, whose Radix Tooltip needs
a TooltipProvider ancestor. The app shell (layout.tsx) provides one in
prod, but page.test.tsx renders <StudiesPage> in isolation — so it now
wraps in TooltipProvider (mirroring the layout). That ed5ca27 didn't
already carry this fix confirms it never ran the full suite on main.

types.ts conflict resolved to main's generated version (the fields are
already present + freshness-gate-canonical). ceiling-badge fixture
conflict merged (trial_count + convergence_verdict both present).

1039 vitest green; tsc + build clean; freshness gate clean (no backend
change). No migration.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
…base)

state_history.md + the feat_studies_convergence_visibility implementation
plan both marked the frontend Trials + Convergence columns as shipped via
PR #421 (commit ed5ca27). They did not — ed5ca27 was dropped in the
#421/#422 de-duplication rebase; only the backend fields landed. Annotate
both with a CORRECTION pointing at the 2026-06-03 restoration
(feat_studies_list_trial_convergence_columns).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
…umns

The /studies list now carries Trials + Convergence columns (restored in
this PR), so guide 06's 01-studies-list.png was stale. Regenerated
against the populated stack (make up + make seed-demo): the list
screenshot now shows the TRIALS column (50/200/200/15/...) + the
CONVERGENCE column (CONVERGED / TOO FEW TRIALS badges) with their header
tooltips. 03-create-study-modal (list behind the modal) + 04-study-detail
(a richer completed study) regenerated in lockstep; walkthrough video
promoted.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request restores the frontend 'Trials' and 'Convergence' columns in the /studies list that were lost during a previous rebase. It adds the column configurations, glossary tooltips, unit tests, and an E2E test, while also wrapping the page in a TooltipProvider to prevent Radix tooltip errors. The review feedback suggests adding a defensive check in the column configuration to handle unmapped convergence verdicts gracefully during rolling deployments, and importing ReactNode explicitly in the new test file to avoid referencing the global React namespace.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +161 to +172
const badge = VERDICT_BADGE[verdict];
return (
<span
className="inline-flex items-center"
data-testid={`convergence-verdict-${row.original.id}`}
data-verdict={verdict}
>
<Badge variant={badge.variant} className="text-[10px] uppercase tracking-wide">
{badge.label}
</Badge>
</span>
);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

During rolling deployments or API updates, the backend might return a new or unexpected convergence_verdict value that is not yet mapped in VERDICT_BADGE. To prevent runtime crashes (e.g., TypeError: Cannot read properties of undefined (reading 'variant')), add a defensive check to handle cases where badge is undefined.

      const badge = VERDICT_BADGE[verdict];
      if (!badge) return <span className="text-muted-foreground">—</span>;
      return (
        <span
          className="inline-flex items-center"
          data-testid={`convergence-verdict-${row.original.id}`}
          data-verdict={verdict}
        >
          <Badge variant={badge.variant} className="text-[10px] uppercase tracking-wide">
            {badge.label}
          </Badge>
        </span>
      );

Comment on lines +16 to +18
import { render, screen } from '@testing-library/react';
import { describe, expect, it } from 'vitest';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Import ReactNode from 'react' to type-safe the cell renderer return type and avoid referencing the global React namespace.

Suggested change
import { render, screen } from '@testing-library/react';
import { describe, expect, it } from 'vitest';
import { render, screen } from '@testing-library/react';
import { type ReactNode } from 'react';
import { describe, expect, it } from 'vitest';

if (!column?.cell || typeof column.cell !== 'function') {
throw new Error(`${columnId} column or its cell renderer not found`);
}
const cell = column.cell as (ctx: { row: { original: StudySummary } }) => React.ReactNode;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Use the imported ReactNode type directly instead of referencing React.ReactNode without React in scope.

Suggested change
const cell = column.cell as (ctx: { row: { original: StudySummary } }) => React.ReactNode;
const cell = column.cell as (ctx: { row: { original: StudySummary } }) => ReactNode;

All three findings accepted:

1. column-config.tsx — forward-compat guard `if (!badge) return <em-dash>`
   for an unmapped convergence_verdict. Unlike the unreachable
   declared_params NULL case (rejected on PR #436 — DB NOT NULL), this is
   REACHABLE: convergence_verdict is a backend-COMPUTED classification
   (backend/app/domain/study/convergence.py), not a fixed DB enum, so a
   newer backend could emit a verdict this snapshot doesn't map during a
   rolling deploy — and an unmapped value would throw on `badge.variant`,
   crashing the whole table render. Matches the codebase's existing
   forward-compat stance (StatusBadge `?? 'secondary'`; the best_metric
   column's rolling-deploy comment in this same file). + regression test
   (cast an out-of-union verdict → asserts em-dash, no crash).

2+3. convergence-column.test.tsx — import `type ReactNode` from 'react'
   explicitly instead of the ambient `React.ReactNode` namespace; matches
   the rest of the codebase's import style.

8/8 convergence-column tests green; tsc + 0 lint errors.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
@SoundMindsAI
Copy link
Copy Markdown
Owner Author

Review adjudication (Gemini Code Assist)

Commit landing accepted fixes: 85eb14fb

Gemini Code Assist (3 findings — all accepted)

# Sev Location Verdict Notes
1 Medium studies-table.column-config.tsx:172 Accepted Forward-compat guard if (!badge) return <em-dash> for an unmapped convergence_verdict. Distinct from the declared_params or {} finding I rejected on PR #436: that was a NOT-NULL DB column → unreachable. This is REACHABLE — convergence_verdict is a backend-computed classification (backend/app/domain/study/convergence.py), not a fixed DB enum, so a newer backend could emit a verdict this frontend snapshot doesn't map during a rolling deploy, and badge.variant on the undefined lookup would crash the whole table render. Matches the codebase's existing forward-compat pattern (StatusBadge VARIANT_TABLE[...]?.[value] ?? 'secondary'; the best_metric column's rolling-deploy backward-compat comment in this same file). Added a regression test (cast an out-of-union verdict → asserts em-dash fallback, no crash).
2 Medium studies-table-convergence-column.test.tsx:18 Accepted Import type ReactNode from 'react' explicitly rather than referencing the ambient React.ReactNode namespace. Matches the rest of the codebase's import style.
3 Medium studies-table-convergence-column.test.tsx:45 Accepted Use the imported ReactNode directly (companion to #2).

Outcomes

  • Applied fixes (3): forward-compat badge guard + regression test; explicit ReactNode import (×2).
  • Rejected (0) · Deferred (0)

Note: this PR also corrects a doc-drift bug it uncovered — the studies-list columns were marked "shipped" in PR #421 but were actually lost in the #421/#422 rebase (see PR description + the CORRECTION annotations in state_history.md + the feature plan).

All 17 CI checks were green on bddf3570; the 85eb14fb fixes are frontend-only (re-running). Ready for human review + merge once green.

@SoundMindsAI
Copy link
Copy Markdown
Owner Author

Final cross-model review (GPT-5.5) — adjudication

GPT-5.5 returned 1 Low-severity finding, rejected:

# Sev Location Verdict Notes
1 Low studies-table.column-config.tsx (badge lookup) Rejected "A verdict named __proto__/constructor/toString returns a truthy inherited method from VERDICT_BADGE[verdict], bypassing the if (!badge) guard." Unreachable in practice: convergence_verdict is a backend-computed classification with three fixed branches (backend/app/domain/study/convergence.pyconverged/still_improving/too_few_trials) — it never emits a JS-prototype-method name. The failure mode is a cosmetic empty badge, not a crash. And the codebase's own StatusBadge uses the identical plain-index pattern (VARIANT_TABLE[...]?.[value] ?? 'secondary') with the same characteristic — hardening only here with Object.hasOwn would diverge from that precedent. If prototype-safe enum lookups are desired, that's a codebase-wide convention change (StatusBadge included), not a one-off in this PR. The accepted Gemini #1 guard already handles the reachable case (a real new verdict value → undefined → em-dash).

Outcomes (all reviews on this PR)

Ready for human review + merge once CI is green on 85eb14fb.

@SoundMindsAI SoundMindsAI merged commit 03976c5 into main Jun 3, 2026
18 checks passed
@SoundMindsAI SoundMindsAI deleted the feat_studies_list_trial_convergence_columns branch June 3, 2026 22:16
SoundMindsAI added a commit that referenced this pull request Jun 3, 2026
…438) (#439)

Prepend the #438 entry to Last 5 merges (noting it restored the lost
PR #421 Story 1.2 columns + corrected the doc-drift); drop the now-6th
entry to the older-entries pointer; update branch context + in-flight to
reflect all three 2026-06-03 features merged.

Signed-off-by: SoundMindsAI <eric.starr@soundminds.ai>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant